fix: /cd command now reloads agent context including AGENTS.md#240
Open
wkramme wants to merge 8 commits intompfaffenberger:mainfrom
Open
fix: /cd command now reloads agent context including AGENTS.md#240wkramme wants to merge 8 commits intompfaffenberger:mainfrom
wkramme wants to merge 8 commits intompfaffenberger:mainfrom
Conversation
Fixes issue where /cd command would change the working directory but the agent context would not be refreshed, causing: - AGENTS.md files in new directory to be ignored - Stale working directory in system prompts - Plugin callbacks not re-firing with new context Changes: - Modified /cd command to clear _puppy_rules cache and reload agent - Added agent reload with graceful error handling - Added two comprehensive tests to verify reload behavior This ensures that when users change directories, the agent picks up project-specific AGENTS.md rules and refreshes all context-dependent prompt components.
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughModifies the Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as "CLI (/cd)"
participant OS as "OS Filesystem"
participant AgentMgr as "Agent Manager"
participant Logger as "Emitter"
CLI->>OS: expanduser/isabs/isdir -> chdir
alt chdir successful
CLI->>AgentMgr: get_current_agent()
alt agent exists
AgentMgr->>AgentMgr: reload_code_generation_agent()
AgentMgr-->>CLI: success
CLI->>Logger: emit_info("Agent context updated for new directory")
else no agent
CLI->>Logger: emit_info("No agent to reload")
end
else chdir failed
CLI->>Logger: emit_error("Directory change failed")
end
alt reload raises exception
AgentMgr-->>CLI: exception
CLI->>Logger: emit_error("Could not reload agent context: <error>")
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
code_puppy/command_line/core_commands.py (1)
87-91: Avoid mutating_puppy_rulesfrom the command layer.At Line 89, this reaches into agent internals and duplicates behavior already done inside
reload_code_generation_agent()(which invalidates_puppy_rulesitself). Keeping invalidation inside the agent reduces coupling.♻️ Proposed simplification
current_agent = get_current_agent() - if current_agent: - # Clear cached puppy rules so AGENTS.md is re-read from new directory - current_agent._puppy_rules = None - # Reload agent to rebuild system prompt with new working directory - current_agent.reload_code_generation_agent() - emit_info("Agent context updated for new directory") + # reload_code_generation_agent() already invalidates cached rules + # and rebuilds prompt/context from the new cwd. + current_agent.reload_code_generation_agent() + emit_info("Agent context updated for new directory")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@code_puppy/command_line/core_commands.py` around lines 87 - 91, Remove the direct mutation of the agent internals: delete the line that sets current_agent._puppy_rules = None and rely on current_agent.reload_code_generation_agent() to handle invalidation; keep the comment about reloading if needed but do not touch _puppy_rules from the command layer and ensure reload_code_generation_agent() is the single place that clears or rebuilds puppy rules.tests/command_line/test_core_commands_extended.py (1)
159-160: This assertion is coupled to a private implementation detail.At Line 159, asserting
_puppy_rulesdirectly makes the test brittle to internal refactors. Prefer validating public behavior (reload_code_generation_agentcalled and/cdsucceeds), which you already assert.🧪 Proposed test hardening
- # Verify agent cache was cleared - assert mock_agent._puppy_rules is None - # Verify agent was reloaded mock_agent.reload_code_generation_agent.assert_called_once()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/command_line/test_core_commands_extended.py` around lines 159 - 160, The test currently asserts the private field mock_agent._puppy_rules which couples the test to an implementation detail; instead remove that private-field assertion and verify public behavior: assert that mock_agent.reload_code_generation_agent was called (or use the public method/flag it sets) and that the subsequent "/cd" command succeeds as already checked; update the assertion to use mock_agent.reload_code_generation_agent (or the public API it triggers) rather than accessing _puppy_rules directly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@code_puppy/command_line/core_commands.py`:
- Around line 87-91: Remove the direct mutation of the agent internals: delete
the line that sets current_agent._puppy_rules = None and rely on
current_agent.reload_code_generation_agent() to handle invalidation; keep the
comment about reloading if needed but do not touch _puppy_rules from the command
layer and ensure reload_code_generation_agent() is the single place that clears
or rebuilds puppy rules.
In `@tests/command_line/test_core_commands_extended.py`:
- Around line 159-160: The test currently asserts the private field
mock_agent._puppy_rules which couples the test to an implementation detail;
instead remove that private-field assertion and verify public behavior: assert
that mock_agent.reload_code_generation_agent was called (or use the public
method/flag it sets) and that the subsequent "/cd" command succeeds as already
checked; update the assertion to use mock_agent.reload_code_generation_agent (or
the public API it triggers) rather than accessing _puppy_rules directly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ce2d58b0-0313-48eb-8bc7-b3000efff6f8
📒 Files selected for processing (2)
code_puppy/command_line/core_commands.pytests/command_line/test_core_commands_extended.py
- Removed direct mutation of agent._puppy_rules from /cd command - reload_code_generation_agent() now handles invalidation internally - Updated test to verify public behavior only (reload called) - Removed brittle assertion on private _puppy_rules field This reduces coupling and makes the code more maintainable.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/command_line/test_core_commands_extended.py (1)
137-159: LGTM, but remove redundant import.The test correctly verifies that
/cdtriggers agent reload. The mocking strategy is sound—patchingget_current_agentat the source module works since the import happens at call-time inside the implementation's try block.Minor:
MagicMockis already imported at line 8; the local import on line 139 is unnecessary.Suggested fix
def test_cd_reloads_agent_context(self): """Test that /cd reloads agent to pick up new directory context (AGENTS.md, etc).""" - from unittest.mock import MagicMock - mock_agent = MagicMock()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/command_line/test_core_commands_extended.py` around lines 137 - 159, Remove the redundant local import of MagicMock in the test_cd_reloads_agent_context test: delete the line that re-imports MagicMock (the second import) and rely on the existing MagicMock imported at the top of the file; ensure the test still instantiates mock_agent = MagicMock() and calls handle_cd_command("/cd /new/dir") and verifies mock_agent.reload_code_generation_agent.assert_called_once().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/command_line/test_core_commands_extended.py`:
- Around line 137-159: Remove the redundant local import of MagicMock in the
test_cd_reloads_agent_context test: delete the line that re-imports MagicMock
(the second import) and rely on the existing MagicMock imported at the top of
the file; ensure the test still instantiates mock_agent = MagicMock() and calls
handle_cd_command("/cd /new/dir") and verifies
mock_agent.reload_code_generation_agent.assert_called_once().
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 677aa899-1aee-4e55-bd5b-e0ef0105468f
📒 Files selected for processing (2)
code_puppy/command_line/core_commands.pytests/command_line/test_core_commands_extended.py
🚧 Files skipped from review as they are similar to previous changes (1)
- code_puppy/command_line/core_commands.py
MagicMock is already imported at the top of the test file. Removed local re-imports in test_cd_reloads_agent_context and test_cd_handles_agent_reload_failure_gracefully.
The /cd command now uses emit_error for reload failures (not emit_warning). Also applied ruff formatting for consistency.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/test_command_handler.py (1)
93-122:⚠️ Potential issue | 🟠 MajorPatch cleanup mismatch leaks
emit_errormock across tests.You start
emit_errorat Line 93 but never stop it infinally. This can create cross-test interference.Suggested fix
finally: mocks["emit_success"].stop() - mocks["emit_warning"].stop() + mocks["emit_error"].stop()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_command_handler.py` around lines 93 - 122, The test starts the emit_error mock (mock_emit_error = mocks["emit_error"].start()) but never stops it in the finally block, leaking the mock across tests; update the finally block to call mocks["emit_error"].stop() (matching how mocks["emit_success"] and mocks["emit_warning"] are stopped) so that mock_emit_error is properly torn down after this test (affecting the test around handle_command and assertions on mock_agent.reload_code_generation_agent).
🧹 Nitpick comments (1)
tests/command_line/test_core_commands_extended.py (1)
141-158: Strengthen success-path assertion for the new info message.This test patches
emit_infobut never verifies it. Since/cdnow reports context refresh, assert that the info emission happened to lock the behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/command_line/test_core_commands_extended.py` around lines 141 - 158, The test currently patches code_puppy.messaging.emit_info but doesn't assert it was called; update the test around handle_cd_command to capture the patched emit_info (e.g., assign the patch to a variable like mock_emit_info) and add an assertion that mock_emit_info was invoked (preferably assert_called_once() or assert_called_once_with(...) if you know the exact message) to verify the new "/cd" success path emits the context-refresh info; reference the patched symbol emit_info and the function under test handle_cd_command (and keep the existing assertion that mock_agent.reload_code_generation_agent.assert_called_once()).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@code_puppy/command_line/core_commands.py`:
- Around line 93-95: The message emitted when reload fails uses emit_error(...)
but begins with the literal "Warning:", causing inconsistent severity; update
the call where emit_error is invoked for the reload failure (the string
"Warning: Could not reload agent context: {e}") to remove the "Warning:" prefix
(e.g., "Could not reload agent context: {e}") so the text matches the emit_error
severity, or alternatively call emit_warning(...) instead—modify the emit_* call
and its message accordingly to make severity consistent.
---
Outside diff comments:
In `@tests/test_command_handler.py`:
- Around line 93-122: The test starts the emit_error mock (mock_emit_error =
mocks["emit_error"].start()) but never stops it in the finally block, leaking
the mock across tests; update the finally block to call
mocks["emit_error"].stop() (matching how mocks["emit_success"] and
mocks["emit_warning"] are stopped) so that mock_emit_error is properly torn down
after this test (affecting the test around handle_command and assertions on
mock_agent.reload_code_generation_agent).
---
Nitpick comments:
In `@tests/command_line/test_core_commands_extended.py`:
- Around line 141-158: The test currently patches code_puppy.messaging.emit_info
but doesn't assert it was called; update the test around handle_cd_command to
capture the patched emit_info (e.g., assign the patch to a variable like
mock_emit_info) and add an assertion that mock_emit_info was invoked (preferably
assert_called_once() or assert_called_once_with(...) if you know the exact
message) to verify the new "/cd" success path emits the context-refresh info;
reference the patched symbol emit_info and the function under test
handle_cd_command (and keep the existing assertion that
mock_agent.reload_code_generation_agent.assert_called_once()).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: db398ec7-0b87-45c6-957b-84583354e8aa
📒 Files selected for processing (3)
code_puppy/command_line/core_commands.pytests/command_line/test_core_commands_extended.pytests/test_command_handler.py
- Fix mock cleanup leak (stop emit_error instead of emit_warning) - Remove 'Warning:' prefix from emit_error message for consistency - Add assertion for emit_info call in test_cd_reloads_agent_context - Update test to expect new error message format (no 'Warning:' prefix)
Applied black-compatible formatting to 3 test files for consistency.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/test_command_handler.py (1)
117-120: Tighten the reload-error assertion to avoid dead/overly-broad matching.
code_puppy/command_line/core_commands.pyemits"Could not reload agent context: ..."on this path, so the"agent reload failed"branch is effectively dead here. Also, asserting onstr(call_args)is looser than checking the emitted message argument directly.Proposed test tightening
- error_msg = str(mock_emit_error.call_args) - assert ( - "agent reload failed" in error_msg - or "Could not reload agent context" in error_msg - ) - assert "boom" in error_msg + error_msg = mock_emit_error.call_args[0][0] + assert error_msg.startswith("Could not reload agent context:") + assert "boom" in error_msg🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_command_handler.py` around lines 117 - 120, Tighten the test assertion by removing the redundant "agent reload failed" branch and assert the actual emitted log message argument directly: inspect the mock call (call_args) to the logger and assert its first positional argument contains the exact prefix "Could not reload agent context:" (the message emitted by code_puppy/command_line/core_commands.py) instead of asserting on str(call_args) or using the loose error_msg membership check; update the test variable usage (error_msg/call_args) accordingly so it checks call_args.args[0] (or equivalent) for the expected message prefix.tests/command_line/test_core_commands_extended.py (1)
185-188: Strengthen failure-path assertions to lock in diagnostics.Consider asserting exactly one error emission and that the original exception text is present. That makes the non-fatal error contract more robust.
Proposed assertion hardening
- assert mock_error.called - error_msg = mock_error.call_args[0][0] - assert "Could not reload agent context" in error_msg + mock_error.assert_called_once() + error_msg = mock_error.call_args[0][0] + assert error_msg.startswith( + "Could not reload agent context:" + ) + assert "Reload failed" in error_msg🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/command_line/test_core_commands_extended.py` around lines 185 - 188, The test currently checks that mock_error was called and that "Could not reload agent context" is in the logged message; strengthen this by asserting mock_error.call_count == 1 to ensure exactly one error emission, and also assert that the original exception text is present in error_msg (use the same exception message you inject in the failure scenario, e.g., the stringified exception variable used when simulating the reload failure) so both the diagnostic and call-count contract are locked in; reference mock_error and error_msg when adding these assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/command_line/test_core_commands_extended.py`:
- Around line 185-188: The test currently checks that mock_error was called and
that "Could not reload agent context" is in the logged message; strengthen this
by asserting mock_error.call_count == 1 to ensure exactly one error emission,
and also assert that the original exception text is present in error_msg (use
the same exception message you inject in the failure scenario, e.g., the
stringified exception variable used when simulating the reload failure) so both
the diagnostic and call-count contract are locked in; reference mock_error and
error_msg when adding these assertions.
In `@tests/test_command_handler.py`:
- Around line 117-120: Tighten the test assertion by removing the redundant
"agent reload failed" branch and assert the actual emitted log message argument
directly: inspect the mock call (call_args) to the logger and assert its first
positional argument contains the exact prefix "Could not reload agent context:"
(the message emitted by code_puppy/command_line/core_commands.py) instead of
asserting on str(call_args) or using the loose error_msg membership check;
update the test variable usage (error_msg/call_args) accordingly so it checks
call_args.args[0] (or equivalent) for the expected message prefix.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 012a5ed5-83ef-4621-8d48-8d437b685f52
📒 Files selected for processing (5)
code_puppy/command_line/core_commands.pytests/agents/test_base_agent_full_coverage.pytests/command_line/test_core_commands_extended.pytests/command_line/test_model_settings_menu_coverage.pytests/test_command_handler.py
✅ Files skipped from review due to trivial changes (2)
- tests/agents/test_base_agent_full_coverage.py
- tests/command_line/test_model_settings_menu_coverage.py
🚧 Files skipped from review as they are similar to previous changes (1)
- code_puppy/command_line/core_commands.py
Per CodeRabbit review: - Use assert_called_once() instead of assert called - Check exact prefix with startswith() - Assert original exception text is present - Use call_args[0][0] instead of str(call_args) - Remove redundant 'agent reload failed' dead code check
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The
/cdcommand changes the working directory but the agent context was not being refreshed, causing:Root Causes
_puppy_rules- AGENTS.md content is cached inbase_agent.pyand never re-read/cd- Directory changes but agent context stays staleChanges
/cdcommand to clear_puppy_rulescache and reload agentImpact
When users
/cdto a new directory, the agent now:on_load_prompt)Testing
All 7
/cdcommand tests pass, including 2 new tests:test_cd_reloads_agent_contexttest_cd_handles_agent_reload_failure_gracefullySummary by CodeRabbit
Bug Fixes
/cdnow triggers an automatic agent context reload after changing directories.Tests
/cdand graceful handling when reload raises an exception; updated expectations to match the new error emission behavior.